Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Linux: Add lsblk plugin #1239

Open
wants to merge 3 commits into
base: develop
Choose a base branch
from

Conversation

dalonjon
Copy link

This pull request adds a plugin that mimics the linux lsblk utility. It loops through the block devices on the machine and prints out the device name, major, minor, removability, if the device is read only, type and mountpoint.

Tested on linux versions 3.13-6.8 with Ubuntu, Rhel, Centos, Debian, sles, and Oracle Linux distros. Worked with the latest version for each distro.

Here is an example invocation of this plugin:
`% ./vol.py -f data6.5-RaidTest.lime lsblk
Volatility 3 Framework 2.6.1
Progress: 100.00 Stacking attempts finished
NAME MAJOR MINOR RM RO SIZE TYPE MOUNTPOINT

loop0 7 0 0 1 4.0K loop /snap/bare/5
loop1 7 1 0 1 74.2M loop /snap/core22/1122
loop2 7 2 0 1 266.6M loop /snap/firefox/3836
loop3 7 3 0 1 497.0M loop /snap/gnome-42-2204/141
loop4 7 4 0 1 74.2M loop /snap/core22/1439
loop5 7 5 0 1 268.4M loop /snap/firefox/4650
loop6 7 6 0 1 12.3M loop /snap/snap-store/959
loop7 7 7 0 1 40.4M loop /snap/snapd/20671
sdb 8 16 0 0 32.0G disk
sda 8 0 0 0 32.0G disk
sdc 8 32 0 0 25.0G disk
sdb1 8 17 0 0 32.0G part
sdc1 8 33 0 0 25.0G part
sda1 8 1 0 0 1.0M part
sda2 8 2 0 0 513.0M part /boot/efi
sda3 8 3 0 0 31.5G part /
md0 9 0 0 0 25.0G raid1
sr0 11 0 1 0 1024.0M rom
loop8 7 8 0 1 91.7M loop /snap/gtk-common-themes/1535
loop10 7 10 0 1 452.0K loop /snap/snapd-desktop-integration/83
loop9 7 9 0 1 38.8M loop /snap/snapd/21759`

This pull request adds a plugin that mimics the linux lsblk utility. It
loops through the block devices on the machine and prints out the device
name, major, minor, removability, if the device is read only, type and
mountpoint.

Tested on linux versions 3.13-6.8 with Ubuntu, Rhel, Centos, Debian, sles, and
Oracle Linux distros. Worked with the latest version for each distro.
Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks very much for your submission, there's quite a lot going on so I can't guarantee these are all the comments to be made, but it should be enough to be getting on with. There is at least one show stopper, where some results are converted to strings before being returned from the plugin, which is bad from the perspective of allowing other tools to use the data this plugin returns. That will need to change before this is accepted, but most everything else looks pretty good. There's a lot of backtracking from a subtype to a parent type, which ok, but could do with a lot of documentation so people can see/verify what's going on.

volatility3/framework/plugins/linux/lsblk.py Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved

return utility.array_to_string(gendisk.disk_name)

def _format_bytes(self, size):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Feels like this could be used for more than just the linux.lsblk function, consider adding it as a staticmethod somewhere more common across plugins. As mentioned above, this is really a UI function rather than a plugin function. The plugin should return the raw data, and let the UI decide what needs to happen to display it properly (using a type hint to say how you'd expect it to be displayed).

volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
return f"{size / 1024 ** 3:.1f}G"
return f"{size / 1024 ** 4:.1f}T"

def _get_type(self, vmlinux, block_device, gendisk):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Again, type information and ideally context first and simpler parameters where possible afterwards.

SCSI device types. Copied almost as-is from kernel header
(include/scsi/scsi_proto.h)
"""
type_map = {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is kinda ok here, but if anything else ever would be able to make use of it, it should be lifted out into constants.linux somewhere (if it doesn't already exist as an enumeration in linux symbol tables).

volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
@ikelos ikelos requested a review from atcuno August 20, 2024 20:56
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
volatility3/framework/plugins/linux/lsblk.py Outdated Show resolved Hide resolved
Copy link
Member

@ikelos ikelos left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looking much better and the code is much easier to read through now, thanks! Just a few more points but looking really close to the finish line. 5:)

from volatility3.framework.objects import utility
from volatility3.framework.symbols import linux
from volatility3.framework.renderers import format_hints
from volatility3.framework.constants.linux import GOLDEN_RATIO_PRIME_BEFORE_4_7, GOLDEN_RATIO_PRIME_AFTER_4_7
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Generally we import modules, rather than specific values, so that if people find it in this code and are lazy, they can't import this plugin more easily than importing the module they need. So please import either constants or linux and then use either constants.linux.GOLDEN... or linux.GOLDEN... respectively (I think I'd prefer constants).

return utility.array_to_string(gendisk.disk_name)

def _get_type(self, context, block_device, gendisk):
vmlinux = context.modules[self.config["kernel"]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rather than limiting this function to being usable during a run of this plugin, could we add a parameter for kernel_module_name please? This would then allow a lot of these to become classmethods, and make then usable by other plugins in the future.

return inode.i_bdev

def _device_iterator(self, context, klist_devices):
vmlinux = context.modules[self.config["kernel"]]
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please could we parameterize this on kernel_module_name rather than going to the config? It'll make the function more usable outside of the bounds of just this plugin.

device = linux.LinuxUtilities.container_of(klist_node.vol.offset, "device", "knode_class", vmlinux)
yield device

def _get_name(self, vmlinux, block_device, gendisk):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, rather than passing in vmlinux, the convention for volatility plugins is to pass in the context as the first parameter and then the module name as the second, and then lookup the module in context.modules. Sticking to the convention should make it easier if/when we come to parallelize things rather than passing around lots of objects, it's just the context and then a reference for something inside the context.


return inode.i_bdev

def _device_iterator(self, context, klist_devices):
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to provide basic docstring describing what these functions do and what their input arguments are and what output they produce please?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could this helper make it in the core API please ? This can definitely be useful for any kind of device parser plugin 👍

@ikelos ikelos self-requested a review September 10, 2024 21:01
Comment on lines +42 to +46
try:
class_kset = vmlinux.object_from_symbol("class_kset")
except exceptions.SymbolError:
class_kset = None
if not class_kset:
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Please use vmlinux.has_symbol() instead of all these

Comment on lines +63 to +85
# Before v5.11, partitions are represented by hd_struct instead of block_device
if not vmlinux.has_type("hd_struct"):
block_device = linux.LinuxUtilities.container_of(device.vol.offset, "block_device", "bd_device", vmlinux)
gendisk = block_device.bd_disk

# lsblk by default skips over ram devices
try:
if utility.array_to_string(gendisk.disk_name).startswith("ram"):
continue
except exceptions.InvalidAddressException:
continue
else:
hd_struct = linux.LinuxUtilities.container_of(device.vol.offset, "hd_struct", "__dev", vmlinux)
gendisk = linux.LinuxUtilities.container_of(hd_struct.vol.offset, "gendisk", "part0", vmlinux)
try:
if utility.array_to_string(gendisk.disk_name).startswith("ram"):
continue
except exceptions.InvalidAddressException:
continue

block_device = self._get_block_device(vmlinux, device.devt)
if not block_device:
continue
Copy link
Contributor

@gcmoreira gcmoreira Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It seems like you could refactor this into an internal helper function, such as _get_block_device(), which would return either the block_device or None. This would help make the large for statement more readable and maintainable.

Copy link
Contributor

@gcmoreira gcmoreira Oct 8, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oops, I just noticed you already have a _get_block_device() method, but it seems to only work for kernels 5.11 and up, which adds to the confusion. You'll need to rethink how to refactor this.
Ideally, there should be a single _get_block_device() method, and based on the kernel version (the type checks you are doing in the above lines), it can either call the method for kernels >= 5.11 or the one for < 5.11. This would make the logic much clearer.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants